- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.1k
Make coverage more similar to the one in Scala 2 #23722
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
          
     Merged
      
      
    
                
     Merged
            
            
          Conversation
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
    fb9a3c7    to
    da43ab4      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good.
I didn't fully understand one part of the PR, so I left a question about that.
        
          
                compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala
              
                Outdated
          
            Show resolved
            Hide resolved
        
      * remove coverage of inlined nodes (as mentioned in the accompanying comment, those are impossible to represent in most cases) * add coverage for Literals (ones directly in Apply are omitted) * remove coverage of `throw` contents * if apply node is tagged, do not tag it's prefix, outside of other prefixing Apply's arguments (eg. when we tag `a+b+c` we do not redundantly tag `a+b`) * allow instrumenting synthetic method calls (like apply of a case class) Also fixes issue with certain generated Block nodes not having assigned the correct type
da43ab4    to
    2c479e1      
    Compare
  
    
              
                    KacperFKorban
  
              
              approved these changes
              
                  
                    Aug 15, 2025 
                  
              
              
            
            
    
  tgodzik 
      pushed a commit
        to scala/scala3-lts
      that referenced
      this pull request
    
      Sep 1, 2025 
    
    
      
  
    
      
    
  
Closes scala#21877 * removes coverage of inlined nodes (as mentioned in the accompanying comment, those are impossible to represent in most cases) * adds coverage for Literals (ones directly in Apply are omitted) * removes coverage of `throw` contents * if apply node is tagged, we do not tag it's prefix, outside of other prefixing Apply's arguments (eg. when we tag `a+b+c` we do not redundantly tag `a+b`) * allows instrumenting synthetic method calls (like apply of a case After all of these changes the statements tagged are much more similar to Scala 2, let's look at the scala#21877 minimisation: * Scala 2: <img width="704" height="364" alt="Zrzut ekranu 2025-08-12 o 17 07 31" src="https://github.com/user-attachments/assets/f647dfa5-973e-424f-9818-483b7d01d550" /> <img width="740" height="379" alt="Zrzut ekranu 2025-08-12 o 17 07 46" src="https://github.com/user-attachments/assets/09eca1c0-a202-4e5e-b3e4-0947d9e8662d" /> * Scala 3: <img width="623" height="360" alt="Zrzut ekranu 2025-08-12 o 17 08 48" src="https://github.com/user-attachments/assets/efd5baaa-9f52-4ad6-9ba6-2f5bde42a470" /> <img width="638" height="428" alt="Zrzut ekranu 2025-08-12 o 17 08 55" src="https://github.com/user-attachments/assets/01ff6cc6-c348-47db-8ae5-d758ca0bf302" /> There are some differences still remaining, most notably the tagging the DefDefs and its default parameters, but I left them for now, as those seem more useful than harmful. BEcouse of those changed most of the .covergae files had to be regenerated, however I want through each and every diff to make sure that all of those changes there are expected. Additionally, this PR also fixes scala#21695 (issue with certain generated Block nodes not having assigned the correct type, causing later undefined errors).
    
  tgodzik 
      added a commit
        to scala/scala3-lts
      that referenced
      this pull request
    
      Sep 1, 2025 
    
    
      
  
    
      
    
  
Closes scala#21877 * removes coverage of inlined nodes (as mentioned in the accompanying comment, those are impossible to represent in most cases) * adds coverage for Literals (ones directly in Apply are omitted) * removes coverage of `throw` contents * if apply node is tagged, we do not tag it's prefix, outside of other prefixing Apply's arguments (eg. when we tag `a+b+c` we do not redundantly tag `a+b`) * allows instrumenting synthetic method calls (like apply of a case After all of these changes the statements tagged are much more similar to Scala 2, let's look at the scala#21877 minimisation: * Scala 2: <img width="704" height="364" alt="Zrzut ekranu 2025-08-12 o 17 07 31" src="https://github.com/user-attachments/assets/f647dfa5-973e-424f-9818-483b7d01d550" /> <img width="740" height="379" alt="Zrzut ekranu 2025-08-12 o 17 07 46" src="https://github.com/user-attachments/assets/09eca1c0-a202-4e5e-b3e4-0947d9e8662d" /> * Scala 3: <img width="623" height="360" alt="Zrzut ekranu 2025-08-12 o 17 08 48" src="https://github.com/user-attachments/assets/efd5baaa-9f52-4ad6-9ba6-2f5bde42a470" /> <img width="638" height="428" alt="Zrzut ekranu 2025-08-12 o 17 08 55" src="https://github.com/user-attachments/assets/01ff6cc6-c348-47db-8ae5-d758ca0bf302" /> There are some differences still remaining, most notably the tagging the DefDefs and its default parameters, but I left them for now, as those seem more useful than harmful. BEcouse of those changed most of the .covergae files had to be regenerated, however I want through each and every diff to make sure that all of those changes there are expected. Additionally, this PR also fixes scala#21695 (issue with certain generated Block nodes not having assigned the correct type, causing later undefined errors). [Cherry-picked c535dbc][modified]
    
  WojciechMazur 
      pushed a commit
        to scala/scala3-lts
      that referenced
      this pull request
    
      Sep 9, 2025 
    
    
      
  
    
      
    
  
Closes scala#21877 * removes coverage of inlined nodes (as mentioned in the accompanying comment, those are impossible to represent in most cases) * adds coverage for Literals (ones directly in Apply are omitted) * removes coverage of `throw` contents * if apply node is tagged, we do not tag it's prefix, outside of other prefixing Apply's arguments (eg. when we tag `a+b+c` we do not redundantly tag `a+b`) * allows instrumenting synthetic method calls (like apply of a case After all of these changes the statements tagged are much more similar to Scala 2, let's look at the scala#21877 minimisation: * Scala 2: <img width="704" height="364" alt="Zrzut ekranu 2025-08-12 o 17 07 31" src="https://github.com/user-attachments/assets/f647dfa5-973e-424f-9818-483b7d01d550" /> <img width="740" height="379" alt="Zrzut ekranu 2025-08-12 o 17 07 46" src="https://github.com/user-attachments/assets/09eca1c0-a202-4e5e-b3e4-0947d9e8662d" /> * Scala 3: <img width="623" height="360" alt="Zrzut ekranu 2025-08-12 o 17 08 48" src="https://github.com/user-attachments/assets/efd5baaa-9f52-4ad6-9ba6-2f5bde42a470" /> <img width="638" height="428" alt="Zrzut ekranu 2025-08-12 o 17 08 55" src="https://github.com/user-attachments/assets/01ff6cc6-c348-47db-8ae5-d758ca0bf302" /> There are some differences still remaining, most notably the tagging the DefDefs and its default parameters, but I left them for now, as those seem more useful than harmful. BEcouse of those changed most of the .covergae files had to be regenerated, however I want through each and every diff to make sure that all of those changes there are expected. Additionally, this PR also fixes scala#21695 (issue with certain generated Block nodes not having assigned the correct type, causing later undefined errors).
    
  WojciechMazur 
      pushed a commit
        to scala/scala3-lts
      that referenced
      this pull request
    
      Sep 9, 2025 
    
    
      
  
    
      
    
  
Closes scala#21877 * removes coverage of inlined nodes (as mentioned in the accompanying comment, those are impossible to represent in most cases) * adds coverage for Literals (ones directly in Apply are omitted) * removes coverage of `throw` contents * if apply node is tagged, we do not tag it's prefix, outside of other prefixing Apply's arguments (eg. when we tag `a+b+c` we do not redundantly tag `a+b`) * allows instrumenting synthetic method calls (like apply of a case After all of these changes the statements tagged are much more similar to Scala 2, let's look at the scala#21877 minimisation: * Scala 2: <img width="704" height="364" alt="Zrzut ekranu 2025-08-12 o 17 07 31" src="https://github.com/user-attachments/assets/f647dfa5-973e-424f-9818-483b7d01d550" /> <img width="740" height="379" alt="Zrzut ekranu 2025-08-12 o 17 07 46" src="https://github.com/user-attachments/assets/09eca1c0-a202-4e5e-b3e4-0947d9e8662d" /> * Scala 3: <img width="623" height="360" alt="Zrzut ekranu 2025-08-12 o 17 08 48" src="https://github.com/user-attachments/assets/efd5baaa-9f52-4ad6-9ba6-2f5bde42a470" /> <img width="638" height="428" alt="Zrzut ekranu 2025-08-12 o 17 08 55" src="https://github.com/user-attachments/assets/01ff6cc6-c348-47db-8ae5-d758ca0bf302" /> There are some differences still remaining, most notably the tagging the DefDefs and its default parameters, but I left them for now, as those seem more useful than harmful. BEcouse of those changed most of the .covergae files had to be regenerated, however I want through each and every diff to make sure that all of those changes there are expected. Additionally, this PR also fixes scala#21695 (issue with certain generated Block nodes not having assigned the correct type, causing later undefined errors). [Cherry-picked c535dbc][modified]
    
  tgodzik 
      pushed a commit
        to tgodzik/scala3
      that referenced
      this pull request
    
      Sep 18, 2025 
    
    
      
  
    
      
    
  
Closes scala#21877 * removes coverage of inlined nodes (as mentioned in the accompanying comment, those are impossible to represent in most cases) * adds coverage for Literals (ones directly in Apply are omitted) * removes coverage of `throw` contents * if apply node is tagged, we do not tag it's prefix, outside of other prefixing Apply's arguments (eg. when we tag `a+b+c` we do not redundantly tag `a+b`) * allows instrumenting synthetic method calls (like apply of a case After all of these changes the statements tagged are much more similar to Scala 2, let's look at the scala#21877 minimisation: * Scala 2: <img width="704" height="364" alt="Zrzut ekranu 2025-08-12 o 17 07 31" src="https://github.com/user-attachments/assets/f647dfa5-973e-424f-9818-483b7d01d550" /> <img width="740" height="379" alt="Zrzut ekranu 2025-08-12 o 17 07 46" src="https://github.com/user-attachments/assets/09eca1c0-a202-4e5e-b3e4-0947d9e8662d" /> * Scala 3: <img width="623" height="360" alt="Zrzut ekranu 2025-08-12 o 17 08 48" src="https://github.com/user-attachments/assets/efd5baaa-9f52-4ad6-9ba6-2f5bde42a470" /> <img width="638" height="428" alt="Zrzut ekranu 2025-08-12 o 17 08 55" src="https://github.com/user-attachments/assets/01ff6cc6-c348-47db-8ae5-d758ca0bf302" /> There are some differences still remaining, most notably the tagging the DefDefs and its default parameters, but I left them for now, as those seem more useful than harmful. BEcouse of those changed most of the .covergae files had to be regenerated, however I want through each and every diff to make sure that all of those changes there are expected. Additionally, this PR also fixes scala#21695 (issue with certain generated Block nodes not having assigned the correct type, causing later undefined errors).
    
  tgodzik 
      added a commit
      that referenced
      this pull request
    
      Sep 19, 2025 
    
    
  
    
  WojciechMazur 
      pushed a commit
      that referenced
      this pull request
    
      Sep 22, 2025 
    
    
      
  
    
      
    
  
Closes #21877 * removes coverage of inlined nodes (as mentioned in the accompanying comment, those are impossible to represent in most cases) * adds coverage for Literals (ones directly in Apply are omitted) * removes coverage of `throw` contents * if apply node is tagged, we do not tag it's prefix, outside of other prefixing Apply's arguments (eg. when we tag `a+b+c` we do not redundantly tag `a+b`) * allows instrumenting synthetic method calls (like apply of a case After all of these changes the statements tagged are much more similar to Scala 2, let's look at the #21877 minimisation: * Scala 2: <img width="704" height="364" alt="Zrzut ekranu 2025-08-12 o 17 07 31" src="https://github.com/user-attachments/assets/f647dfa5-973e-424f-9818-483b7d01d550" /> <img width="740" height="379" alt="Zrzut ekranu 2025-08-12 o 17 07 46" src="https://github.com/user-attachments/assets/09eca1c0-a202-4e5e-b3e4-0947d9e8662d" /> * Scala 3: <img width="623" height="360" alt="Zrzut ekranu 2025-08-12 o 17 08 48" src="https://github.com/user-attachments/assets/efd5baaa-9f52-4ad6-9ba6-2f5bde42a470" /> <img width="638" height="428" alt="Zrzut ekranu 2025-08-12 o 17 08 55" src="https://github.com/user-attachments/assets/01ff6cc6-c348-47db-8ae5-d758ca0bf302" /> There are some differences still remaining, most notably the tagging the DefDefs and its default parameters, but I left them for now, as those seem more useful than harmful. BEcouse of those changed most of the .covergae files had to be regenerated, however I want through each and every diff to make sure that all of those changes there are expected. Additionally, this PR also fixes #21695 (issue with certain generated Block nodes not having assigned the correct type, causing later undefined errors). [Cherry-picked c535dbc]
  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment
  
      
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
Closes #21877
throwcontentsa+b+cwe do not redundantly taga+b)After all of these changes the statements tagged are much more similar to Scala 2, let's look at the #21877 minimisation:
There are some differences still remaining, most notably the tagging the DefDefs and its default parameters, but I left them for now, as those seem more useful than harmful.
BEcouse of those changed most of the .covergae files had to be regenerated, however I want through each and every diff to make sure that all of those changes there are expected.
Additionally, this PR also fixes #21695 (issue with certain generated Block nodes not having assigned the correct type, causing later undefined errors).